feat(macos/capture): enable EDR (HDR) output on macOS 14+ for 10-bit pixel formats#5191
feat(macos/capture): enable EDR (HDR) output on macOS 14+ for 10-bit pixel formats#5191Nottlespike wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new macOS screen-capture backend using ScreenCaptureKit (macOS 12.3+) and wires it into the existing display capture pipeline while keeping the legacy AVCaptureScreenInput path for older macOS versions.
Changes:
- Introduce
SCVideo(ScreenCaptureKit-based) as an alternative capture implementation conforming to a shared protocol. - Update
display.mmto select betweenSCVideoandAVVideoat runtime and to store the capture object behindid<SunshineVideoCapture>. - Update CMake to link
ScreenCaptureKitand compilesc_video.mwith ARC.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/platform/macos/sc_video.m | New ScreenCaptureKit-based capture implementation (ARC) with dynamic range handling and callback-based frame delivery. |
| src/platform/macos/sc_video.h | Declares SCVideo and aligns it with the shared capture protocol used by callers. |
| src/platform/macos/display.mm | Switch capture backend by OS availability and adapt NV12 device initialization/type handling. |
| src/platform/macos/av_video.h | Introduces SunshineVideoCapture protocol shared by AVVideo and SCVideo. |
| cmake/dependencies/macos.cmake | Adds ScreenCaptureKit framework discovery. |
| cmake/compile_definitions/macos.cmake | Links ScreenCaptureKit, adds new sources, and enables ARC for sc_video.m. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.streamConfig.width = self.frameWidth; | ||
| self.streamConfig.height = self.frameHeight; |
| CGDisplayModeRelease(mode); | ||
| } | ||
|
|
||
| self.sampleQueue = dispatch_queue_create("dev.lizardbyte.sunshine.sckCapture", dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_USER_INTERACTIVE, DISPATCH_QUEUE_PRIORITY_HIGH)); |
| [SCShareableContent getShareableContentExcludingDesktopWindows:NO | ||
| onScreenWindowsOnly:NO | ||
| completionHandler:^(SCShareableContent *_Nullable content, NSError *_Nullable error) { |
| } | ||
| dispatch_semaphore_signal(ready); | ||
| }]; | ||
| dispatch_semaphore_wait(ready, DISPATCH_TIME_FOREVER); |
| if (!self.streamRunning) { | ||
| NSError *outputError = nil; | ||
| if (![self.stream addStreamOutput:self | ||
| type:SCStreamOutputTypeScreen | ||
| sampleHandlerQueue:self.sampleQueue | ||
| error:&outputError]) { |
| if (self.streamRunning) { | ||
| [self.stream stopCaptureWithCompletionHandler:^(NSError *_Nullable error) { | ||
| (void) error; | ||
| }]; | ||
| self.streamRunning = NO; | ||
| } |
| if (@available(macOS 14.0, *)) { | ||
| if ([SCVideo pixelFormatIsHighBitDepth:pixelFormat]) { |
| self.streamConfig.captureDynamicRange = SCCaptureDynamicRangeHDRLocalDisplay; | ||
| } else { | ||
| self.streamConfig.captureDynamicRange = SCCaptureDynamicRangeSDR; | ||
| } | ||
| } |
| "${CMAKE_SOURCE_DIR}/src/platform/macos/sc_video.h" | ||
| "${CMAKE_SOURCE_DIR}/src/platform/macos/sc_video.m" |
| set_source_files_properties( | ||
| "${CMAKE_SOURCE_DIR}/src/platform/macos/sc_video.m" | ||
| PROPERTIES COMPILE_FLAGS "-fobjc-arc") |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88208f69c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (![self.stream addStreamOutput:self | ||
| type:SCStreamOutputTypeScreen | ||
| sampleHandlerQueue:self.sampleQueue | ||
| error:&outputError]) { |
There was a problem hiding this comment.
capture: adds self as an SCStream output every time streamRunning is false, but the stop paths only call stopCapture and never remove that output, so restarts reuse a stream with stale output registration. This is exercised by the current flow (e.g., dummy_img warm-up then real capture in src/video.cpp), and can cause the second start to fail (addStreamOutput returns NO) or to deliver duplicated callbacks if the framework accepts duplicate registrations.
Useful? React with 👍 / 👎.
| if (startError) { | ||
| NSLog(@"SCVideo: startCapture failed: %@", startError); | ||
| dispatch_semaphore_signal(self.currentSignal); | ||
| return self.currentSignal; |
There was a problem hiding this comment.
On addStreamOutput/startCapture failure, this method signals and returns a semaphore instead of propagating an error, and display.mm subsequently waits and returns capture_e::ok unconditionally. That means startup failures are reported as successful capture sessions, which can leave the pipeline in a silent no-frame state instead of triggering error/reinit handling.
Useful? React with 👍 / 👎.
Nottlespike
force-pushed
the
feat/macos/capture/edr-hdr-output
branch
from
88208f6 to
50b75e8
Compare
|
Most of the inline suggestions are duplicated from #5190 and have been addressed there (this PR's diff includes #5190's commit; once that lands the diff reduces to just the EDR commit). Specifically resolved in the amended SCK commit:
EDR-specific:
Force-pushed both branches. |
|
…12.3+
AVCaptureScreenInput was deprecated in macOS 13 (October 2022) and is
fundamentally limited to 8-bit BGRA, blocking any honest HDR or 10-bit
work on the macOS capture path. ScreenCaptureKit has been available
since macOS 12.3 (March 2022) and is the only forward path; this
commit lays the foundation by adding a drop-in SCK-based backend that
preserves behaviour exactly (same pixel format, frame rate, display
selection) so it can be reviewed independently of the HDR work that
builds on top.
Changes:
* Add SunshineVideoCapture protocol in av_video.h declaring the
capture-side surface both backends expose.
* Make AVVideo conform to the protocol (no behaviour change; pure
declaration).
* Add SCVideo (sc_video.h / sc_video.m) implementing the same
protocol against SCStream + SCContentFilter + SCStreamConfiguration.
Built with -fobjc-arc for SCK's block-heavy API surface; objects
cross the MRC boundary via the standard +1-retain alloc/init
convention so display.mm continues to work in MRC.
* Drop incomplete frames from SCK output by inspecting
SCStreamFrameInfoStatus on each sample-buffer attachment, matching
the reliability the legacy path got for free from AVCaptureSession.
* display.mm now holds an id<SunshineVideoCapture> and branches at
construction via @available(macOS 12.3, *): SCVideo on supported
systems, AVVideo as fallback for older macOS.
* Wire ScreenCaptureKit framework into cmake/dependencies/macos.cmake
and cmake/compile_definitions/macos.cmake; set ARC compile flag on
sc_video.m only.
Pixel format stays 32BGRA for this commit; 10-bit + EDR metadata
follow in a subsequent change.
…pixel formats
With AVCaptureScreenInput, asking the capture surface for a 10-bit
pixel format silently produced 8-bit BGRA — the OS-level lie that
made HEVC Main10 / AV1 Main10 / ProRes 10-bit profiles on macOS into
fake HDR (color-tagged 8-bit data). With ScreenCaptureKit landing in
the previous commit, 10-bit pixel formats are actually honoured, but
SCK needs an explicit signal to attach HDR metadata to those buffers
instead of treating them as 10-bit Rec.709.
This commit wires SCStreamConfiguration.captureDynamicRange:
* Add +pixelFormatIsHighBitDepth: classifier covering the YUV 4:2:0,
4:2:2 and 4:4:4 10-bit BiPlanar formats plus ARGB2101010 packed
and 64-bit RGBA formats.
* On the synchronous init path, set captureDynamicRange immediately
if the starting pixel format is high bit depth so the very first
sample buffer carries HDR metadata.
* On the setPixelFormat: path (called by nv12_zero_device when the
encoder selects p010), also update captureDynamicRange and push
the new config to a running stream via -updateConfiguration:.
* Use SCCaptureDynamicRangeHDRLocalDisplay rather than canonical
HDR: game streaming wants the host display's actual HDR
characteristics (peak luminance, primaries) so the receiver shows
what a local user would see, not Apple's idealised reference.
* Guard the whole block behind @available(macOS 14.0, *); on
12.3-13.x SCK still honours the 10-bit pixel format request but
doesn't auto-tag buffers, so Sunshine's existing colorspace logic
continues to drive the encoder's color fields.
Validated on M4 Max: Sunshine's encoder probe matrix now includes
successful 10-bit HEVC and 10-bit ProRes entries that previously
could not have validated because the capture surface couldn't
deliver matching pixel data. ProRes-specific VideoToolbox color tags
land in a separate follow-up commit.
Nottlespike
force-pushed
the
feat/macos/capture/edr-hdr-output
branch
from
50b75e8 to
fa43fb1
Compare
|
Force-pushed (rebased on new #5190 tip). Same Re Copilot's latest inline pass: all six suggestions it surfaced are already fixed in the rebased branch, just at shifted line numbers post-amend. Quick map:
The Copilot bot doesn't re-evaluate inline comments after force-pushes; the suggestions are valid critiques of the pre-amend code that's no longer present in the branch. |
|
Closing and folding into #5190. Per the 'each PR must be mergeable in any order' guidance, the EDR work is textually dependent on the SCK API surface in #5190 (it would not compile against the legacy AVCaptureScreenInput path), so splitting it across two PRs is artificial. The EDR commit will be squashed onto #5190 along with the HDR-gating hardening (gating EDR on negotiated |
Folding in the requested changes that came in during the upstream review cycle and weren't yet on dev. None of these change runtime behaviour for the cases the existing code already handled; they harden the surface against future-codec additions, encoder-constraint demotion, missing SDK paths, and out-of-tree readers of the games-category rationale. video.h: is_known_video_format() now uses `< SUNSHINE_FORMAT_COUNT` instead of the hardcoded `<= SUNSHINE_FORMAT_PRORES` upper bound. Adding a future codec is now just bumping the enum — no need to remember this predicate. (Copilot, LizardByte#5192 inline @ src/video.h:34) video.cpp: require_prores now binds to the immutable config::video.prores_mode source rather than the mutable active_prores_mode global. adjust_encoder_constraints() can demote active_prores_mode to 0 when no encoder supports ProRes; reading from the immutable source keeps the "user explicitly asked for forced ProRes" intent intact across that demotion, so we fail loudly instead of silently picking a non-ProRes encoder. (Copilot, LizardByte#5192 inline @ src/video.cpp:2882/2945/2978) cmake/dependencies/macos.cmake: SCREEN_CAPTURE_KIT_LIBRARY now uses FIND_LIBRARY(... REQUIRED) with an explanatory comment. Sunshine compiles sc_video.m unconditionally, so any build environment lacking the SCK framework needs to fail at configure time with a clear message, not later at header lookup. (Copilot, LizardByte#5191 inline @ cmake/compile_definitions/macos.cmake:60/71) src_assets/macos/build/Info.plist.in: Inlined the two most authoritative public references for the gamepolicyd category-string coupling (Apple Developer Forums thread 739387 and moonlight-qt#1297). Anyone re-evaluating this plist entry in the future has both pointers without going through PR archaeology. (Copilot, LizardByte#5189 inline @ Info.plist.in:28)
ReenigneArcher
added
the
ai
Description
With
AVCaptureScreenInput, asking the capture surface for a 10-bit pixel format silently produced 8-bit BGRA — an OS-level lie that made HEVC Main10 / AV1 Main10 / future ProRes 10-bit profiles on macOS into fake HDR (color-tagged 8-bit data). WithScreenCaptureKitlanding in #5190, 10-bit pixel formats are actually honoured, but SCK needs an explicit signal (captureDynamicRange) to attach HDR metadata to those buffers instead of treating them as 10-bit Rec.709.Changes:
+pixelFormatIsHighBitDepth:classifier inSCVideocovering the YUV 4:2:0 / 4:2:2 / 4:4:4 10-bit BiPlanar formats plusARGB2101010packed and 64-bit RGBA formats. Returning YES is the signal that the capture surface is HDR-capable.captureDynamicRangeimmediately if the starting pixel format is high bit depth, so the very first sample buffer carries HDR metadata (no first-frame-SDR flash).setPixelFormat:path (called bynv12_zero_devicewhen the encoder selectspix_fmt_e::p010): also updatescaptureDynamicRangeand pushes the new config to a running stream via-updateConfiguration:.SCCaptureDynamicRangeHDRLocalDisplayrather than the canonical HDR mode: game streaming wants the host display's actual HDR characteristics (peak luminance, primaries) so the receiver shows what a local user would see, not Apple's idealised reference encoding.@available(macOS 14.0, *)guard: on 12.3-13.x SCK still honours the 10-bit pixel format request fromnv12_zero_devicebut the OS doesn't auto-tag buffers; Sunshine's existing colorspace logic continues to drive the encoder's color fields in that case.Validated on M4 Max (macOS 14+):
Sunshine's encoder probe matrix at startup now contains:
Previously these 10-bit probes could not validate because the capture surface couldn't deliver matching 10-bit data —
validate_configwould fail at encoder init time. With this change, real 10-bit data flows through the pipeline for the first time on macOS, and the per-codecDYNAMIC_RANGEcapability bit is set honestly based on actual probe results rather than optimistic.capabilities.set().What this does NOT yet do (separate follow-ups):
kVTCompressionPropertyKey_ColorPrimaries/_TransferFunction/_YCbCrMatrix) — ProRes carries color in the frame header rather than SEI, so this is its own discrete change.Encoder side is already wired generically (Sunshine sets
ctx->color_primaries / color_trc / colorspacefromavcodec_colorspaceatsrc/video.cpp:1781-1783for all encoders), so HEVC Main10 HDR via VideoToolbox should now produce honest BT.2020 PQ output for any HDR-requesting client.Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage